[terra-arrange][terra-section-header] update to utilize semantic html elements #3831

KV106606Viswanath · 2023-06-26T06:36:34Z

Summary

Currently Toggle Section Header wraps a H2 with a button which might not be valid HTML. While the screen reader does read and interpret code correctly, the invalid code could fail for other browsers or assistive technologies. Added valid html to avoid these failures.

What was changed:
Updated HTML code div elements with button and span tags to make it valid html.

Why it was changed:
To avoid failures of invalid html on other browsers or assistive technologies.

This PR resolves:

UXPLATFORM-9093

Thank you for contributing to Terra.
@cerner/terra

sdadn

You may need to update jest snapshots as well.

sdadn · 2023-06-26T18:39:20Z

packages/terra-arrange/CHANGELOG.md

@@ -2,6 +2,9 @@

 ## Unreleased

+* Changed
+  * Updated div elements with span tag elements.


Suggested change

* Updated div elements with span tag elements.

* Updated Arrange to use `span` elements instead of `div`.

sdadn · 2023-06-26T18:40:48Z

packages/terra-section-header/CHANGELOG.md

+* Added
+  * Added a function "ArrangeComponent" to use Arrange component.


CHANGELOGs are for consumers so we don't have to add entries for internal implementation changes.

Suggested change

* Added

* Added a function "ArrangeComponent" to use Arrange component.

Removed these 2 lines.

sdadn · 2023-06-26T19:21:34Z

packages/terra-section-header/CHANGELOG.md

+  * Updated div element with span tag element.
+  * Updated div element with button tag element.


Suggested change

* Updated div element with span tag element.

* Updated div element with button tag element.

* Updated SectionHeader to use `span` & `button` elements instead of `div`.

supreethmr · 2023-06-27T19:13:28Z

packages/terra-arrange/src/Arrange.module.scss

+.fit-block {
+  .icon-wrapper {
+    display: inline-flex;
+    font-size: 8px;


Are these style values provided by UX..??

No, these are not provided by UX team. I have added them to match the existing look of UI. If required i will check with UX team.

yes we need UX review for the above style changes !!

Got +1 from UX

supreethmr · 2023-06-27T19:15:17Z

packages/terra-arrange/src/Arrange.jsx

-      </div>
-      <div {...fillProps} className={cx('fill', align || alignFill, fillProps.className)}>
+      </span>
+      <span {...fillProps} className={cx('fill', align || alignFill, fillProps.className || 'fill-block')}>


Suggested change

<span {...fillProps} className={cx('fill', align || alignFill, fillProps.className || 'fill-block')}>

<span {...fillProps} className={cx('fill', align || alignFill, fillProps.className, 'fill-block')}>

why are using || here with fillProps.classname ?

Removed that || operator as it's not required here.

supreethmr · 2023-06-27T19:17:14Z

packages/terra-arrange/src/Arrange.module.scss

@@ -37,3 +37,16 @@
    align-self: flex-start;
  }
 }
+
+.fit-block {
+  .icon-wrapper {


Is this class intended for terra-arrange ? there are no reference in Arrange component for class icon-wrapper !!

This class has been added in section header component, i have moved these styles into section header module scss file.

sdadn · 2023-07-07T20:29:47Z

packages/terra-core-docs/CHANGELOG.md

Can you rebase with main so that this file is up to date?

sdadn · 2023-07-07T20:35:27Z

packages/terra-section-header/src/SectionHeader.jsx

+    const ArrangeComponent = () => (
+      <Arrange
+        fitStart={onClick && accordionIcon}
+        fill={<span aria-hidden={(onClick !== undefined)} className={cx('title', 'title-block')}>{headerText}</span>}
+        className={cx('title-arrange')}
+      />
+    );


This can be a variable instead of a function.

Removed this function and added Arrange component directly inside ArrangeWrapperTag as part of latest updates.

sycombs · 2023-07-07T20:38:57Z

packages/terra-section-header/src/SectionHeader.jsx

+      />
+    );
+
+    const buttonAttributes = (onClick) ? { 'aria-expanded': isOpen, 'aria-label': headerText } : undefined;


Are these still needed? Should aria-expanded': isOpen be set on the <button> and 'aria-label': headerText be set on the <span>?

I realized I misread this. I don't think we need the ternary anymore since we can set { 'aria-expanded': isOpen, 'aria-label': headerText } directly on the <button>.

sycombs · 2023-07-07T20:42:57Z

Why is the text color different?

https://github.com/cerner/terra-core/blob/0a838433201102146b09c06c762cfe6218f55f11/packages/terra-section-header/tests/wdio/__snapshots__/reference/clinical-lowlight-theme/en/chrome_huge/section-header-spec/long_title_accordion_default.png

sdadn · 2023-07-07T20:46:47Z

packages/terra-section-header/src/SectionHeader.jsx

+          <button {...buttonAttributes} type="button" tabIndex="-1" className={cx('arrange-wrapper toggle-button')}>
+            <ArrangeComponent />
+          </button>
+        ) : (
+          <span tabIndex="-1" className={cx('arrange-wrapper')}>
+            <ArrangeComponent />
+          </span>


Both of these seem very similar so instead of defining these twice, we can do something like:

const ArrangeWrapperTag = onClick ? "button" : "span" ; const type = onClick ? "button" : null; // ... and so on for the other attributes <ArrangeWrapperTag (attributes) > <ArrangeComponent /> </ArrangeWrapperTag

sycombs · 2023-07-26T18:22:47Z

packages/terra-core-docs/CHANGELOG.md

+* Added
+  * Added realistic examples for `terra-arrange`.
+  * Added realistic examples for `terra-toggle-section-header`.
+
 * Added
  * Added email validation for `terra-form-field`.


Let's combine the Added sections:

Suggested change

* Added

* Added realistic examples for `terra-arrange`.

* Added realistic examples for `terra-toggle-section-header`.

* Added

* Added email validation for `terra-form-field`.

* Added

* Added realistic examples for `terra-arrange`.

* Added realistic examples for `terra-toggle-section-header`.

* Added email validation for `terra-form-field`.

sycombs · 2023-07-26T18:23:16Z

packages/terra-core-docs/CHANGELOG.md

@@ -40,7 +47,6 @@
 * Added
  * Added terra-scroll A11y tests.
  * Added an example in terra-content-container without interactive elements.
-  * Added examples for terra-content-container with dark colors.


Why was this removed?

This entry was now Added.

sycombs · 2023-07-26T18:28:00Z

packages/terra-toggle-section-header/README.md

@@ -4,7 +4,7 @@
 [![NPM version](https://badgen.net/npm/v/terra-toggle-section-header)](https://www.npmjs.org/package/terra-toggle-section-header)
 [![Build Status](https://badgen.net/travis/cerner/terra-core)](https://travis-ci.com/cerner/terra-core)

-Toggle section header component that transitions content in and out with a click.
+Toggle section header component that transitions content in and out with a click. For accessibility best practices, it is recommended that consumers should always use only one h1 tag per page or view. The one h1 tag should be the page title. A section header should never be a heading level 1. 


Since this is already in the terra-toggle-section-header About doc, can we remove it from the README? Consumers don't generally refer to the README for component guidance.

… elements (#3831) Co-authored-by: ST063655 <ST063655@cerner.net>

KV106606Viswanath requested a review from a team as a code owner June 26, 2023 06:36

github-actions bot temporarily deployed to preview-pr-3831 June 26, 2023 06:36 Destroyed

KV106606Viswanath requested a review from supreethmr June 26, 2023 06:37

KV106606Viswanath self-assigned this Jun 26, 2023

github-actions bot temporarily deployed to preview-pr-3831 June 26, 2023 09:32 Destroyed

github-actions bot temporarily deployed to preview-pr-3831 June 26, 2023 10:00 Destroyed

github-actions bot temporarily deployed to preview-pr-3831 June 26, 2023 13:02 Destroyed

sdadn suggested changes Jun 26, 2023

View reviewed changes

sdadn added 📦 terra-arrange 📦 terra-section-header Accessibility Review Required labels Jun 26, 2023

sdadn changed the title ~~Adding valid html for toggle section header~~ [terra-arrange][terra-section-header] update to utilize semantic html elements Jun 26, 2023

github-actions bot temporarily deployed to preview-pr-3831 June 27, 2023 08:54 Destroyed

github-actions bot temporarily deployed to preview-pr-3831 June 27, 2023 10:40 Destroyed

supreethmr reviewed Jun 27, 2023

View reviewed changes

github-actions bot temporarily deployed to preview-pr-3831 June 28, 2023 10:00 Destroyed

github-actions bot temporarily deployed to preview-pr-3831 July 3, 2023 05:40 Destroyed

github-actions bot temporarily deployed to preview-pr-3831 July 7, 2023 08:14 Destroyed

github-actions bot temporarily deployed to preview-pr-3831 July 7, 2023 09:43 Destroyed

sycombs added the UX Review Required label Jul 7, 2023

sdadn reviewed Jul 7, 2023

View reviewed changes

sycombs reviewed Jul 7, 2023

View reviewed changes

sdadn reviewed Jul 7, 2023

View reviewed changes

github-actions bot temporarily deployed to preview-pr-3831 July 10, 2023 09:08 Destroyed

github-actions bot temporarily deployed to preview-pr-3831 July 10, 2023 09:48 Destroyed

github-actions bot temporarily deployed to preview-pr-3831 July 10, 2023 11:40 Destroyed

KV106606Viswanath added 5 commits July 26, 2023 19:49

Added realistic examples for Toggle section header and Terra Arrange

f4fceda

Resolved js lint errors

c0fd321

Section header code changes

e7abe72

Added tooltip, dotted border for buttons

d4ffd98

Fixed radio button cyclic behaviour issue on safari browser

ae58b44

KV106606Viswanath force-pushed the KV106606-ToggleSectionHeader-9093 branch from 3314e7b to ae58b44 Compare July 26, 2023 18:06

github-actions bot temporarily deployed to preview-pr-3831 July 26, 2023 18:06 Destroyed

sycombs reviewed Jul 26, 2023

View reviewed changes

Removed unnecessary class names in section header

d626eb5

github-actions bot temporarily deployed to preview-pr-3831 July 26, 2023 19:10 Destroyed

Jest snapshot updates

697b00c

github-actions bot temporarily deployed to preview-pr-3831 July 26, 2023 20:01 Destroyed

Removing aria label for Icon buttons as SR reading button name twice

108ae1b

github-actions bot temporarily deployed to preview-pr-3831 July 27, 2023 10:30 Destroyed

Fix for WDIO Issue.

878229e

github-actions bot temporarily deployed to preview-pr-3831 July 28, 2023 04:51 Destroyed

Added function to prevent scroll of anchor tag

3a5fb39

github-actions bot temporarily deployed to preview-pr-3831 July 28, 2023 08:35 Destroyed

Updated import values

0a88d98

github-actions bot temporarily deployed to preview-pr-3831 July 28, 2023 09:46 Destroyed

Changed order of the imports in parseSvg file

95b4603

github-actions bot temporarily deployed to preview-pr-3831 July 28, 2023 10:59 Destroyed

Changed the order of imports in terra icon

fd603a4

github-actions bot temporarily deployed to preview-pr-3831 July 28, 2023 11:42 Destroyed

supreethmr approved these changes Jul 31, 2023

View reviewed changes

saket2403 merged commit aca35f6 into main Aug 1, 2023
21 checks passed

saket2403 deleted the KV106606-ToggleSectionHeader-9093 branch August 1, 2023 07:11

SG067724 pushed a commit that referenced this pull request Aug 8, 2023

[terra-arrange][terra-section-header] update to utilize semantic html…

2c88483

… elements (#3831) Co-authored-by: ST063655 <ST063655@cerner.net>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[terra-arrange][terra-section-header] update to utilize semantic html elements #3831

[terra-arrange][terra-section-header] update to utilize semantic html elements #3831

KV106606Viswanath commented Jun 26, 2023

sdadn left a comment

sdadn Jun 26, 2023

KV106606Viswanath Jun 27, 2023

sdadn Jun 26, 2023

KV106606Viswanath Jun 27, 2023

sdadn Jun 26, 2023

KV106606Viswanath Jun 27, 2023

supreethmr Jun 27, 2023

KV106606Viswanath Jun 28, 2023

supreethmr Jul 6, 2023

KV106606Viswanath Jul 14, 2023

supreethmr Jun 27, 2023

KV106606Viswanath Jun 28, 2023

supreethmr Jun 27, 2023

KV106606Viswanath Jun 28, 2023

sdadn Jul 7, 2023

KV106606Viswanath Jul 10, 2023

sdadn Jul 7, 2023

KV106606Viswanath Jul 10, 2023 •

edited

Loading

sycombs Jul 7, 2023

sycombs Jul 7, 2023

sycombs commented Jul 7, 2023

sdadn Jul 7, 2023

KV106606Viswanath Jul 10, 2023

sycombs Jul 26, 2023

KV106606Viswanath Jul 26, 2023

sycombs Jul 26, 2023

KV106606Viswanath Jul 27, 2023

sycombs Jul 26, 2023

KV106606Viswanath Jul 26, 2023

	* Updated div elements with span tag elements.
	* Updated Arrange to use `span` elements instead of `div`.

		* Added
		* Added a function "ArrangeComponent" to use Arrange component.

		* Updated div element with span tag element.
		* Updated div element with button tag element.

	* Updated div element with span tag element.
	* Updated div element with button tag element.
	* Updated SectionHeader to use `span` & `button` elements instead of `div`.

	<span {...fillProps} className={cx('fill', align \|\| alignFill, fillProps.className \|\| 'fill-block')}>
	<span {...fillProps} className={cx('fill', align \|\| alignFill, fillProps.className, 'fill-block')}>

[terra-arrange][terra-section-header] update to utilize semantic html elements #3831

[terra-arrange][terra-section-header] update to utilize semantic html elements #3831

Conversation

KV106606Viswanath commented Jun 26, 2023

Summary

sdadn left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

KV106606Viswanath Jul 10, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sycombs commented Jul 7, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

KV106606Viswanath Jul 10, 2023 •

edited

Loading